fix: prevent SubClassFinder availability crash#8457
Conversation
getsectiondata with mach_header_64 doesn't compile on 32-bit watchOS device slices (arm64_32, armv7k). Match the only caller, SentrySubClassFinder, which is iOS/tvOS/visionOS only.
|
|
||
| let count = Int(size) / MemoryLayout<UnsafeRawPointer>.size | ||
| return section.withMemoryRebound(to: UnsafeRawPointer.self, capacity: count) { classes in | ||
| (0..<count).map { unsafeBitCast(classes[$0], to: AnyClass.self) } |
There was a problem hiding this comment.
m: I don't expect any issue here, but just double check this works on arm64e due to pointer authentication
There was a problem hiding this comment.
Double-checked. This is safe on arm64e:
- The
unsafeBitCastonly reinterprets the pointer's type (UnsafeRawPointer→AnyClass); it doesn't strip or re-sign anything. These are the exact classref pointers the ObjC runtime itself stores in__objc_classlistand hands to the loader, so they're already correctly formed for the runtime. - We never do hand-rolled pointer authentication/stripping. Every subsequent use goes through the runtime (
class_getSuperclass,class_getName), which authenticates internally. - App Store binaries ship plain arm64, so arm64e mostly matters for the dyld shared cache / system frameworks, which we walk the same way.
Definitive confirmation needs a real arm64e device run (CI sims are arm64/x86_64); tracked as device-only follow-up in the PR.
There was a problem hiding this comment.
Validated safe on arm64e. Reading __objc_classlist raw and unsafeBitCast-ing to AnyClass works on arm64e because dyld applies chained fixups in place at load time — by the time we read the in-memory section the entries are already runtime-correct pointers, so class_getSuperclass/class_getName accept them directly with no ptrauth strip. Confirmed on a physical arm64e device: 44 classrefs read, 31 view controllers swizzled, zero EXC_BAD_ACCESS.
How this was verified with Claude Code (reproducible)
This investigation was done with Claude Code. To re-run it, plug in an A12+ physical device (iPhone XS or newer — arm64e requires A12), open this branch, and paste the following into Claude Code:
Build the
iOS-Swiftsample as arm64e and run it on my connected physical device to check whetherSentryDefaultObjCRuntimeWrapper.classes(forImage:)is safe when it reads PAC-signed__objc_classlistentries. Steps:
- Get the device UDID:
xcrun devicectl list devices(find theconnectedphysicalone).- Build for that device forcing the arm64e slice, using the project's own signing (do NOT override CODE_SIGN_STYLE/DEVELOPMENT_TEAM — the sample already has manual signing configured):
xcodebuild -project Samples/iOS-Swift/iOS-Swift.xcodeproj -scheme iOS-Swift \ -configuration Debug -destination 'id=<UDID>' -derivedDataPath /tmp/arm64e-build \ ARCHS=arm64e ONLY_ACTIVE_ARCH=NO build- Confirm the built slices are actually arm64e:
(Debug builds put the Swift classes — and thuslipo -archs /tmp/arm64e-build/Build/Products/Debug-iphoneos/iOS-Swift.app/iOS-Swift lipo -archs /tmp/arm64e-build/Build/Products/Debug-iphoneos/iOS-Swift.app/iOS-Swift.debug.dylib__objc_classlist— iniOS-Swift.debug.dylib, so that one matters most.)- Install and launch with the console attached:
xcrun devicectl device install app --device <UDID> /tmp/arm64e-build/Build/Products/Debug-iphoneos/iOS-Swift.app xcrun devicectl device process launch --console --device <UDID> io.sentry.sample.iOS-Swift- In the console output, verify: (a)
SentrySubClassFinderlogsFound N number of classes in image: .../iOS-Swift.debug.dylib, (b) it logsThe following UIViewControllers ... will generate automatic transactions: <list of iOS_Swift.* VCs>, and (c) there is noEXC_BAD_ACCESS/SIGSEGV/crash and the process stays alive.If it reads the classes, names them, and swizzles the app's view controllers without crashing, the raw arm64e classref read is safe. If it crashes in
class_getSuperclass/class_getName, a ptrauth-aware read would be required.
Result on an iPhone 12 (A14, iOS 26.5): app + iOS-Swift.debug.dylib + Sentry.framework all arm64e; SentrySubClassFinder read 44 classrefs from the arm64e __objc_classlist, and SentryUIViewControllerSwizzling swizzled 31 UIViewController subclasses (BenchmarkingViewController, CoreDataViewController, nested/mangled ones like _TtCC9iOS_Swift18PageViewController17RedViewController, …). No crash across two launches; process stayed alive at a live UI.
Why this is the right test: the PAC signing of __objc_classlist entries depends on the slice the image was built as, not just the CPU — an arm64 app on an A12+ device has no signed classrefs. Forcing ARCHS=arm64e on the app (and its .debug.dylib) is what actually exercises signed classrefs. CI and the simulator run arm64/x86_64 where ptrauth is a no-op, which is why this needs a device.
The section read binds to mach_header_64, so guard the conditional with arch(arm64) || arch(x86_64) to exclude 32-bit watchOS device slices (arm64_32, armv7k). Covers all slices iOS/tvOS/visionOS ship.
Guard classes(forImage:) with MH_MAGIC_64 before rebinding the header to mach_header_64, which also safely skips a FAT header instead of misreading it. Document that _dyld_get_image_header/_dyld_get_image_name take the dyld loader read lock, so the method must run off the main thread (its only caller already does). Drop the swizzleClassNameExcludes doc paragraph, remove the sample RoomPlan repro scaffolding, and add a regression test driving the real runtime wrapper against the test bundle.
|
Pushed
Changelog now references #8457. Leaving as a draft for a final look before marking ready. |
Lead with the off-main-thread requirement, drop caller references from the doc comments, and compare the image name with strcmp instead of round-tripping the C string through String.
`magic` is the first field of both `mach_header` and `mach_header_64`, so read it on the original pointer and return [] directly instead of funneling nil through the withMemoryRebound closure.
Explain we don't re-read _dyld_image_count per iteration like the CxaThrowSwapper, since we search for one already-loaded image.
SwiftUI was only referenced in comments, never in code.
The defensive early returns (missing header, non-mach_header_64, absent __objc_classlist, image not loaded) were silent, so a class list coming back empty was hard to triage. Add short log lines.
Follow the test<Function>_when<Condition>_should<Expected> naming convention and the arrange-act-assert layout.
classes(forImage:) confirmed safe on a physical arm64e iPhone 12: 44 classrefs read, 31 view controllers swizzled, no EXC_BAD_ACCESS.
ae2d6da to
b83614b
Compare
The old comment was imprecise for the current flow. Explain that the background walk uses only class_getSuperclass and class_getName, which never message the class and so never trigger +initialize, and that we hand names to the main thread where messaging the class is safe.
Rebind the __objc_classlist section to AnyClass? — the honest Swift type of its Class _Nullable entries — and compactMap out nulls, instead of reading non-optional UnsafeRawPointer and unsafeBitCast-ing each entry to AnyClass. A null entry in a malformed section is now skipped instead of producing an invalid non-optional AnyClass (undefined behavior). Extract the section parsing into an internal static helper so tests can exercise the null-entry case, which a real dyld-loaded section can't be made to contain.
Feed the new classes(inSection:size:) helper a crafted buffer shaped like getsectiondata's return value with a null slot in the middle, and assert the null is skipped. A real dyld-loaded __objc_classlist section never contains nulls, so this edge case is unreachable through classes(forImage:).
Store AnyClass instead of class names in SentrySubClassFinder and call the swizzle block with the stored pointer. This drops the NSClassFromString round-trip on the main thread, which realizes the class and could re-trigger the availability-gated realization crash this branch fixes (GH-8152, swiftlang/swift#72657). It also swizzles exactly the class found in the target image instead of whatever a name lookup resolves for duplicate class names.
…ailability-crash # Conflicts: # Sources/Swift/Core/Integrations/Performance/SentrySubClassFinder.swift # Tests/SentryTests/Integrations/Performance/SentrySubClassFinderTests.swift
Reflect the direct class-pointer handoff (no NSClassFromString), the merged origin/main (#8494 early return), the extracted classes(inSection:) helper, and the current test names.
Unremapped __objc_classlist entries (Finding 2) still reach the swizzler; flag it in the wrapper comment and handoff. Add a filter test that documents it does not close the finding. No behavior change.
noahsmartin
left a comment
There was a problem hiding this comment.
Overall approach approach looks good to me, I would just advise testing on all platforms (watchOS, tvOS, etc...) before shipping
📜 Description
Discover
UIViewControllersubclasses without realizing classes. Instead ofobjc_copyClassNamesForImage+NSClassFromString,SentryDefaultObjCRuntimeWrapper.classes(forImage:)reads the image's__objc_classlistMach-O section to get class pointers (dyld-bound, not realized), then reuses the existingclass_getSuperclasssubclass walk. The confirmed view-controller class pointers are handed straight to the swizzle block on the main thread —NSClassFromStringis no longer used (it realizes the class, and could resolve a same-named class from a different image). The swizzling logic itself is unchanged; only class discovery changed.Neither the superclass walk (
class_getSuperclass) norclass_getNamesends a message to the class, so+initializeis never triggered on the background thread — important becauseUIViewControllers assume they run on the main thread.Guarded with
MH_MAGIC_64before rebinding tomach_header_64(also skips FAT headers), gated to 64-bit archs, and documented as off-main-thread only (the_dyld_*calls take the dyld loader lock).💡 Motivation and Context
Fixes #8152. On SDK start, the old code called
NSClassFromStringon every class in the app image, which realizes it. Realizing a Swift class whose metadata references an@available-gated newer-framework type crashes withEXC_BAD_ACCESSon OS versions below that framework's availability (real devices only). Real-world crashers aren't view controllers — SwiftUI gestureCoordinators,RoomPlan/ActivityKitwrappers — so the SDK must avoid realizing unrelated classes. Underlying Swift/ObjC runtime bug: swiftlang/swift#72657.💚 How did you test it?
__objc_classlistenumeration matchesobjc_copyClassNamesForImageacross every loaded image; a section-parsing test covers null entries; a regression test drives the real wrapper through the finder against the test bundle; andtestGettingSubclasses_DoesNotCallInitializerguards that discovery never triggers+initialize.ARCHS=arm64eand ran it on a physical iPhone 12 (A14, iOS 26.5) — 44 classrefs read, 31 view controllers swizzled, zeroEXC_BAD_ACCESS. Confirms the raw classref read is safe on PAC-signed__objc_classlistentries (dyld applies chained fixups in place at load time). Repro steps in this comment.📝 Checklist
You have to check all boxes before merging:
sendDefaultPIIis enabled.